Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert @redwoodjs/web to ESM/CJS dual package #10868

Merged
merged 73 commits into from
Jun 28, 2024

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Jun 21, 2024

This PR changes redwoodjs/web to a cjs/esm dual package!

There's a lot of hackery to get it all working, but some key unusual changes to make it work:

1. Force importing from cjs files of apollo
We do this for prerender primarily, because using the normal imports ends up failing here. Apollo team will be updating their package so that direct cjs imports will have types too (and we can remove ts-expect-error).

When they release Apollo 4.x - they will introduce the exports field, and we will need to remove the CJS imports.

2. Completely disables HelmetProvider in SSR/RSC
Helmet is not actually used in the SSR+ builds, it uses the internal version - PortalHead - to achieve similar results. As before, some features like title templates aren't supported here.

I had to do this because I could not get react-helmet-async to reliably import in all conditions - it either breaks one or the other between legacy and ESM builds (SSR/RSC)

3. I had to alias @redwoodjs/web/apollo in storybook-webpack
I don't understand exactly why, but importing from the CJS version (which webpack correctly picks) does not work in storybook for the Cell success state. It is very likely a dual package hazard, but I can't quite connect the dots as to why changing the import to ESM fixes it.

4. I left the bins as CJS
Since we're building both, I elected to keep it simpler leaving the bins as before

5. Removing babel transforms means we need to import React in all components
We had a babel plugin do autoimports for React, without it we need to make sure all files containing JSX has an import to React.

Other insights useful for anyone else trying to do a dual package conversion:

Red squiggles on existing projects - "cannot import ESM from CJS module"
These don't happen anymore, because we need to generate different types for CJS. See web/build.mts.

Pay attention to the commits
I made a real effort to commit carefully so you can see what changes maybe required to get stuff working. I'll write this up too.


Tasks:

  • Remove "web/dist" imports throughout the framework.
    I will do this in a follow up PR
  • Asset import types

dac09 added 30 commits June 7, 2024 16:48
Rename files without JSX files to just ts
…ckage-cjsonly-new-build

* 'main' of github.com:redwoodjs/redwood: (66 commits)
  chore(testing dbAuth): Add "yes" prompt testing for WebAuthn (redwoodjs#10823)
  chore(testing dbAuth): Mock console.log to silence test output (redwoodjs#10821)
  chore(testing dbAuth): Remove duplicated tests (redwoodjs#10822)
  chore(testing dbAuth): Remove outdated code (redwoodjs#10820)
  chore(testing dbAuth): Consistent naming and fix blue squiggles (redwoodjs#10819)
  chore(testing dbAuth): provide mock implementations (redwoodjs#10818)
  feat(dbAuth): Only suggest dbAuth setup if needed (redwoodjs#10793)
  chore(dbAuth): Fix test for webauthn prompt in `g dbAuth` (redwoodjs#10814)
  fix(dbAuth): Print the correct "post message" after generation (redwoodjs#10813)
  fix(dbauth): Fix spacing issue in task titles (redwoodjs#10811)
  deps(docs): update braces package (redwoodjs#10809)
  chore(deps): bump braces from 3.0.2 to 3.0.3 in /tasks/check (redwoodjs#10808)
  chore(deps): update yarn to v4.3.0 (redwoodjs#10801)
  chore(deps): update dependency tsx to v4.15.2 (redwoodjs#10800)
  chore(deps): update dependency @clerk/types to v3.65.2 (redwoodjs#10795)
  chore(deps): update dependency firebase to v10.12.2 (redwoodjs#10802)
  chore(deps): update dependency rollup to v4.18.0 (redwoodjs#10799)
  chore(deps): update dependency jsdom to v24.1.0 (redwoodjs#10798)
  chore(deps): update dependency @types/vscode to v1.90.0 (redwoodjs#10796)
  chore(deps): update dependency @clerk/clerk-react to v4.32.2 (redwoodjs#10794)
  ...
+ Still use cjs bins
+ Build esm
+ Update yarn.lock so new bins are picked up
@dac09
Copy link
Contributor Author

dac09 commented Jun 25, 2024

Nearly there, apart from cleanup only thing not seemingly working is the Waterfall blog post in storybook-webpack.

The success component gets this error (doesn't render the first level success either). Could be a double package hazard?

Error: You must register a useQuery hook via the `GraphQLHooksProvider`
  at useQuery (/main.iframe.bundle.js:151249:77)
  at NamedCell (/main.iframe.bundle.js:151654:75)

Manual testing on storybook-vite, and on dev shows that it's actually ok.

I'll keep investigating a bit today, but if we're removing storybook-webpack, may not be worth the effort.

@dac09 dac09 changed the title WIP: rwjs/web esm conversion Convert @redwoodjs/web to ESM/CJS dual package Jun 27, 2024
@dac09 dac09 added this to the SSR milestone Jun 27, 2024
@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Jun 27, 2024
@dac09 dac09 marked this pull request as ready for review June 27, 2024 09:14
@dac09 dac09 requested a review from Josh-Walker-GM June 27, 2024 11:06
@dac09
Copy link
Contributor Author

dac09 commented Jun 27, 2024

Ready to start reviewing @Josh-Walker-GM - I still have a bit of cleanup left, but should be good to start looking through :) Thanksssss

Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through everything there isn't anything that sticks out as too unusual. We've talked through some of the changes here and they make sense.

Thanks for documenting the apollo client import cjs/typescript issue as that was something that I hadn't seen before and was a bit different.

It all looks fine and if CI is happy then I too am happy. As you said we will only get better at understanding all this and can tidy up bits and pieces as we do so.

@dac09 dac09 merged commit 3ff2c9e into redwoodjs:main Jun 28, 2024
51 checks passed
@Josh-Walker-GM Josh-Walker-GM modified the milestones: SSR, v8.0.0 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants